Remove setting of extra_data column in DNAAlignFeature table, so that new...

Remove setting of extra_data column in DNAAlignFeature table, so that new attrib adaptor is used exclusively. Port retiring extra_data eval code into AnalysisAdaptor, so that web_data can still be accessed, but eval is localised to a single class and not the entire set of adaptors.

modules/Bio/EnsEMBL/Analysis.pm

@@ -541,7 +541,7 @@ sub displayable {
   Status     : Stable
 
 =cut
-
+# See also _objFromHashref in AnalysisAdaptor for context.
 sub web_data {
     my ($self,$arg) = @_;
 

modules/Bio/EnsEMBL/DBSQL/AnalysisAdaptor.pm

@@ -675,7 +675,16 @@ sub _objFromHashref {
   my $self = shift;
   my $h = shift;
 
-  my $web_data = $h->{web_data} ? $self->get_dumped_data($h->{web_data}) : '';
+  ### This code moved here under protest. Web formatting does not belong with data ###
+  ### Web requires "web_data" for track configuration, but only uses the accessor once
+  ### meaning this eval can retire once the view has been removed. The column has to stay
+  ### but content is mostly accessed by SQL in web-code, not via accessor.
+  my $data = $h->{web_data};
+  $data ||= '';
+  $data =~ s/\n|\r|\f|(\\\\)//g;
+  my $web_data;
+  $web_data = eval($data); # :X execute semi-trustworthy strings on server.
+  ### Deprecation of generic dump_data and get_dumped_data methods from base class means AnalysisAdaptor now needs to supply that by itself
 
   return Bio::EnsEMBL::Analysis->new_fast({
     dbID             => $h->{analysis_id},

modules/Bio/EnsEMBL/DBSQL/DnaAlignFeatureAdaptor.pm

@@ -119,9 +119,9 @@ sub _columns {
             daf.score
             daf.external_db_id
             daf.hcoverage
-	    daf.external_data
-	    exdb.db_name
-	    exdb.db_display_name);
+            daf.external_data
+            exdb.db_name
+            exdb.db_display_name);
 }
 
 
@@ -162,23 +162,18 @@ sub store {
                              hit_start, hit_end, hit_strand, hit_name,
                              cigar_line, analysis_id, score, evalue,
                              perc_ident, external_db_id, hcoverage, external_data)
-     VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"    # 16 arguments
+     VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,NULL)"    # 16 arguments, external data is being removed
   );
 
 FEATURE:
   foreach my $feat (@feats) {
     if ( !ref $feat || !$feat->isa("Bio::EnsEMBL::DnaDnaAlignFeature") )
     {
-      throw("feature must be a Bio::EnsEMBL::DnaDnaAlignFeature,"
-          . " not a ["
-          . ref($feat)
-          . "]." );
+      throw("feature must be a Bio::EnsEMBL::DnaDnaAlignFeature, not a [". ref($feat). "]." );
     }
 
     if ( $feat->is_stored($db) ) {
-      warning( "DnaDnaAlignFeature ["
-          . $feat->dbID()
-          . "] is already stored in this database." );
+      warning( "DnaDnaAlignFeature [".$feat->dbID()."] is already stored in this database." );
       next FEATURE;
     }
 
@@ -228,11 +223,6 @@ FEATURE:
     $sth->bind_param( 13, $feat->percent_id,     SQL_FLOAT );
     $sth->bind_param( 14, $feat->external_db_id, SQL_INTEGER );
     $sth->bind_param( 15, $feat->hcoverage,      SQL_DOUBLE );
-    # Eagle change: also store the extra data, if available
-    my $extra_data;
-    $extra_data = $self->dump_data($feat->extra_data()) if ($feat->extra_data());
-    $sth->bind_param( 16, $extra_data,  SQL_LONGVARCHAR );
-
     $sth->execute();
 
     my $dbId = $self->last_insert_id("${tablename}_id", undef, $tablename);
@@ -312,7 +302,7 @@ sub save {
   my $db = $self->db();
   my $analysis_adaptor = $db->get_AnalysisAdaptor();
 
-  my $sql = qq{INSERT INTO $tablename (seq_region_id, seq_region_start, seq_region_end, seq_region_strand, hit_start, hit_end, hit_strand, hit_name, cigar_line, analysis_id, score, evalue, perc_ident, external_db_id, hcoverage, external_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)};
+  my $sql = qq{INSERT INTO $tablename (seq_region_id, seq_region_start, seq_region_end, seq_region_strand, hit_start, hit_end, hit_strand, hit_name, cigar_line, analysis_id, score, evalue, perc_ident, external_db_id, hcoverage, external_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULL)};
 
   my %analyses = ();
 
@@ -365,8 +355,6 @@ sub save {
     my $seq_region_id;
     ($feat, $seq_region_id) = $self->_pre_store_userdata($feat);
 
-    my $extra_data = $feat->extra_data ? $self->dump_data($feat->extra_data) : '';
-
     $sth->bind_param(1,$seq_region_id,SQL_INTEGER);
     $sth->bind_param(2,$feat->start,SQL_INTEGER);
     $sth->bind_param(3,$feat->end,SQL_INTEGER);
@@ -383,8 +371,6 @@ sub save {
     $sth->bind_param(13,$feat->percent_id,SQL_FLOAT);
     $sth->bind_param(14,$feat->external_db_id,SQL_INTEGER);
     $sth->bind_param(15,$feat->hcoverage,SQL_DOUBLE);
-    $sth->bind_param(16,$extra_data,SQL_LONGVARCHAR);
-
 
     $sth->execute();
     $original->dbID($self->last_insert_id("${tablename}_id", undef, $tablename));
@@ -634,9 +620,6 @@ sub _objs_from_sth {
       $slice = $dest_slice;
     }
 
-    # Inlining the following in the hash causes major issues with 5.16 and messes up the hash 
-    my $evalled_extra_data = $extra_data ? $self->get_dumped_data($extra_data) : '';
-
     # Finally, create the new DnaAlignFeature.
     push( @features,
           $self->_create_feature_fast(
@@ -658,7 +641,7 @@ sub _objs_from_sth {
                'dbID'            => $dna_align_feature_id,
                'external_db_id'  => $external_db_id,
                'hcoverage'       => $hcoverage,
-               'extra_data'      => $evalled_extra_data,
+               'extra_data'      => undef,
                'dbname'          => $external_db_name,
                'db_display_name' => $external_display_db_name
              } ) );

modules/t/analysis.t

@@ -56,7 +56,7 @@ $analysis->description( "some funny description" );
 $analysis->display_label( "and a label" );
 $analysis->displayable( 1 );
 $analysis->created( "2005-10-28 10:28:29");
-$analysis->web_data("blah");
+$analysis->web_data({thing => 'blah'});
 
 ok($analysis);
 
@@ -71,6 +71,7 @@ my $analysis_out = $analysis_ad->fetch_by_logic_name('dummy_analysis');
 ok($analysis_out);
 
 is($analysis_out->db, 'dummy', "Db matches");
+is($analysis_out->web_data->{thing}, 'blah', "Web data correctly extracted from DB via dirty eval");
 
 ok( check_methods( $analysis_out, "db", "db_file", "dbID", "compare",
 		   "logic_name", "parameters", "gff_source", "gff_feature",
@@ -93,7 +94,7 @@ is($analysis_updated->logic_name(), "new_dummy", "Logic name is correct");
 is($analysis_updated->description(), "new description", "Description is correct");
 is($analysis_updated->display_label(), "new label", "Label is correct");
 is($analysis_updated->displayable(), 0, "Displayable is correct");
-is($analysis_updated->web_data(), "blahblah", "Web data is correct");
+is($analysis_updated->web_data(), "blahblah", "Web data is correct"); # Note this does not test the eval output, just the set value.
 
 # now try updating analysis that has no existing description
 $analysis = Bio::EnsEMBL::Analysis->new();