From bc2957b22e54d9672e5a727468b029c490fd51dd Mon Sep 17 00:00:00 2001
From: Kieron Taylor <ktaylor@ebi.ac.uk>
Date: Tue, 4 Oct 2016 15:28:31 +0100
Subject: [PATCH] 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 | 2 +-
modules/Bio/EnsEMBL/DBSQL/AnalysisAdaptor.pm | 11 ++++++-
.../EnsEMBL/DBSQL/DnaAlignFeatureAdaptor.pm | 33 +++++--------------
modules/t/analysis.t | 5 +--
4 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/modules/Bio/EnsEMBL/Analysis.pm b/modules/Bio/EnsEMBL/Analysis.pm
index e8a42dae7b..b326ace347 100755
--- a/modules/Bio/EnsEMBL/Analysis.pm
+++ b/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) = @_;
diff --git a/modules/Bio/EnsEMBL/DBSQL/AnalysisAdaptor.pm b/modules/Bio/EnsEMBL/DBSQL/AnalysisAdaptor.pm
index 978aa438af..0955db6fdd 100755
--- a/modules/Bio/EnsEMBL/DBSQL/AnalysisAdaptor.pm
+++ b/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},
diff --git a/modules/Bio/EnsEMBL/DBSQL/DnaAlignFeatureAdaptor.pm b/modules/Bio/EnsEMBL/DBSQL/DnaAlignFeatureAdaptor.pm
index 3ffb3a90f7..4f678a04dd 100644
--- a/modules/Bio/EnsEMBL/DBSQL/DnaAlignFeatureAdaptor.pm
+++ b/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
} ) );
diff --git a/modules/t/analysis.t b/modules/t/analysis.t
index 17a7e64bdc..c4f4f5ed05 100644
--- a/modules/t/analysis.t
+++ b/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();
--
GitLab