Bug 2054397: Test Perl code for vendor-specific SQL

Bugzilla/DB.pm

@@ -411,7 +411,7 @@ EOT
 
 # List of abstract methods we are checking the derived class implements
 our @_abstract_methods = qw(new sql_regexp sql_not_regexp sql_limit sql_to_days
-  sql_date_format sql_date_math bz_explain
+  sql_date_format sql_date_math sql_date_to_epoch bz_explain
   sql_group_concat);
 
 # This overridden import method will check implementation of inherited classes
@@ -2192,6 +2192,31 @@ Formatted SQL for adding or subtracting a date and some amount of time (scalar)
 
 =back
 
+=item C<sql_date_to_epoch>
+
+=over
+
+=item B<Description>
+
+Outputs SQL syntax for converting a date/time expression to Unix epoch
+seconds.
+
+Abstract method, should be overridden by database specific code.
+
+=item B<Params>
+
+=over
+
+=item C<$date> - date or name of date type column (scalar)
+
+=back
+
+=item B<Returns>
+
+Formatted SQL for Unix epoch seconds (scalar)
+
+=back
+
 =item C<sql_position>
 
 =over

Bugzilla/DB/MariaDB.pm

@@ -228,6 +228,12 @@ sub sql_date_math {
   return "$date $operator INTERVAL $interval $units";
 }
 
+sub sql_date_to_epoch {
+  my ($self, $date) = @_;
+
+  return "UNIX_TIMESTAMP($date)";
+}
+
 sub sql_iposition {
   my ($self, $fragment, $text) = @_;
   return "INSTR($text, $fragment)";

Bugzilla/DB/Mysql.pm

@@ -229,6 +229,12 @@ sub sql_date_math {
   return "$date $operator INTERVAL $interval $units";
 }
 
+sub sql_date_to_epoch {
+  my ($self, $date) = @_;
+
+  return "UNIX_TIMESTAMP($date)";
+}
+
 sub sql_iposition {
   my ($self, $fragment, $text) = @_;
   return "INSTR($text, $fragment)";

Bugzilla/DB/Oracle.pm

@@ -203,6 +203,12 @@ sub sql_date_math {
   return "$date $operator $time_sql";
 }
 
+sub sql_date_to_epoch {
+  my ($self, $date) = @_;
+
+  return "TRUNC(($date - DATE '1970-01-01') * 86400)";
+}
+
 sub sql_position {
   my ($self, $fragment, $text) = @_;
   return "INSTR($text, $fragment)";

Bugzilla/DB/Pg.pm

@@ -155,6 +155,12 @@ sub sql_date_math {
   return "$date $operator $interval * INTERVAL '1 $units'";
 }
 
+sub sql_date_to_epoch {
+  my ($self, $date) = @_;
+
+  return "CAST(EXTRACT(EPOCH FROM $date) AS BIGINT)";
+}
+
 sub sql_string_concat {
   my ($self, @params) = @_;
 

Bugzilla/DB/Sqlite.pm

@@ -235,6 +235,12 @@ sub sql_date_math {
   return "DATETIME($date, '$operator' || $interval || ' $units')";
 }
 
+sub sql_date_to_epoch {
+  my ($self, $date) = @_;
+
+  return "CAST(STRFTIME('%s', $date) AS INTEGER)";
+}
+
 ###############
 # bz_ methods #
 ###############

Bugzilla/Report/SecurityRisk.pm

@@ -280,6 +280,7 @@ sub _build_initial_bugs {
 
 sub _build_events {
   my ($self) = @_;
+  my $dbh = Bugzilla->dbh;
   return [] if !(@{$self->initial_bug_ids});
   my $bug_ids    = join ', ', @{$self->initial_bug_ids};
   my $start_date = $self->start_date->strftime('%Y-%m-%d %H:%M:%S');
@@ -288,8 +289,10 @@ sub _build_events {
             bug_id,
             bug_when,
             field.name AS field_name,
-            CONCAT(removed) AS removed,
-            CONCAT(added) AS added
+      }
+    . $dbh->sql_string_concat('removed') . qq{ AS removed,
+      }
+    . $dbh->sql_string_concat('added') . qq{ AS added
         FROM
             bugs_activity
             JOIN fielddefs AS field ON fieldid = field.id
@@ -299,9 +302,9 @@ sub _build_events {
             AND field.name IN ('keywords' , 'bug_status')
             AND bug_when >= '$start_date'
         GROUP BY bug_id , bug_when , field.name
-    };
+  };
   # Don't use selectall_hashref as it only gets the latest event each bug.
-  my $result = Bugzilla->dbh->selectall_arrayref($query);
+  my $result = $dbh->selectall_arrayref($query);
   my $type   = ArrayRef [Tuple [Int, Str, Str, Str, Str]];
   $type->assert_valid($result);
 

extensions/BMO/lib/Reports/Triage.pm

@@ -314,7 +314,9 @@ sub owners {
                            LEFT JOIN attachments AS attachments_1 ON bugs_1.bug_id = attachments_1.bug_id
                            LEFT JOIN flags AS flags_1 ON bugs_1.bug_id = flags_1.bug_id AND (flags_1.attach_id = attachments_1.attach_id OR flags_1.attach_id IS NULL)
                            LEFT JOIN flagtypes AS flagtypes_1 ON flags_1.type_id = flagtypes_1.id
-                    WHERE  bugs_1.bug_id = bugs.bug_id AND CONCAT(flagtypes_1.name, flags_1.status) = 'needinfo?')))
+                        WHERE  bugs_1.bug_id = bugs.bug_id AND "
+                . $dbh->sql_string_concat('flagtypes_1.name', 'flags_1.status')
+                . " = 'needinfo?')))
                 AND bugs.component_id = ?
        GROUP BY bugs.bug_type");
 

extensions/BMO/lib/Reports/UserActivity.pm

@@ -182,7 +182,9 @@ sub report {
                    'longdesc' AS name,
                    longdescs.bug_id,
                    NULL AS attach_id,
-                   DATE_FORMAT(longdescs.bug_when, '%Y-%m-%d %H:%i:%s') AS ts,
+                   "
+      . $dbh->sql_date_format('longdescs.bug_when', '%Y-%m-%d %H:%i:%s')
+      . " AS ts,
                    '' AS removed,
                    '' AS added,
                    profiles.login_name,

extensions/BugModal/lib/ActivityStream.pm

@@ -358,7 +358,8 @@ sub _add_duplicates_to_stream {
 
   my $sth = $dbh->prepare("
         SELECT longdescs.who,
-               UNIX_TIMESTAMP(bug_when), " . $dbh->sql_date_format('bug_when') . ",
+               " . $dbh->sql_date_to_epoch('bug_when') . ",
+               " . $dbh->sql_date_format('bug_when') . ",
                type,
                extra_data
           FROM longdescs

extensions/ProdCompSearch/lib/WebService.pm

@@ -207,7 +207,7 @@ sub _build_like_order {
   my @terms;
   foreach my $word (split(/[\s,]+/, $query)) {
     push @terms,
-      "CONCAT(products.name, components.name) LIKE "
+      $dbh->sql_string_concat('products.name', 'components.name') . " LIKE "
       . $dbh->quote('%' . $word . '%')
       if $word ne '';
   }

extensions/UserProfile/lib/Util.pm

@@ -247,7 +247,7 @@ sub _activity_by_status {
            AND fieldid = ?
      GROUP BY added
      UNION ALL
-    SELECT CONCAT('RESOLVED/', added) AS status, COUNT(*) AS count
+    SELECT @{[$dbh->sql_string_concat($dbh->quote('RESOLVED/'), 'added')]} AS status, COUNT(*) AS count
       FROM bugs_activity
      WHERE who = ?
            AND fieldid = ?

scripts/nagios_blocker_checker.pl

@@ -184,7 +184,7 @@
   }
 
   my $sql = <<"EOF";
-        SELECT bug_id, bug_severity, UNIX_TIMESTAMP(bugs.creation_ts) AS ts
+      SELECT bug_id, bug_severity, @{[$dbh->sql_date_to_epoch('bugs.creation_ts')]} AS ts
           FROM bugs
          WHERE $where
                AND COALESCE(resolution, '') = ''

t/013db_portability.t

@@ -0,0 +1,101 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+use 5.14.0;
+use strict;
+use warnings;
+
+use lib qw(. lib local/lib/perl5 t);
+
+use Support::Files;
+use File::Find;
+
+use Test::More;
+
+my %seen_file;
+my @files = grep { $_ ne 't/013db_portability.t' && !$seen_file{$_}++ }
+  (@Support::Files::testitems, @Support::Files::test_files);
+
+my @script_files;
+find(
+  sub {
+    return if -d;
+    return unless /\.pl$/;
+    push @script_files, $File::Find::name;
+  },
+  'scripts'
+);
+
+push @files, grep { !$seen_file{$_}++ } @script_files;
+
+my @rules = (
+  {
+    token => qr/\bUNIX_TIMESTAMP\s*\(/i,
+    helper => 'sql_date_to_epoch',
+    message => 'use $dbh->sql_date_to_epoch(...) instead of UNIX_TIMESTAMP()',
+  },
+  {
+    token => qr/\bDATE_FORMAT\s*\(/i,
+    helper => 'sql_date_format',
+    message => 'use $dbh->sql_date_format(...) instead of DATE_FORMAT()',
+  },
+  {
+    token => qr/\bCONCAT\s*\(/i,
+    helper => 'sql_string_concat',
+    message => 'use $dbh->sql_string_concat(...) instead of CONCAT()',
+  },
+  {
+    token => qr/\b(?:POSITION|INSTR|LOCATE)\s*\(/i,
+    helper => 'sql_position/sql_iposition',
+    message => 'use $dbh->sql_position(...) or $dbh->sql_iposition(...) instead',
+  },
+  {
+    token => qr/\bGROUP_CONCAT\s*\(/i,
+    helper => 'sql_group_concat',
+    message => 'use $dbh->sql_group_concat(...) instead of GROUP_CONCAT()',
+  },
+);
+
+my @violations;
+
+foreach my $file (@files) {
+  next if $file =~ m{^Bugzilla/DB(?:/|\.pm$)};
+
+  open(my $fh, '<', $file) or do {
+    push @violations, { file => $file, line => 0, text => 'could not open file' };
+    next;
+  };
+
+  my $line_number = 0;
+  while (my $line = <$fh>) {
+    $line_number++;
+    next if $line =~ /^\s*#/;
+    next if $line =~ /^\s*=/;
+
+    foreach my $rule (@rules) {
+      next unless $line =~ $rule->{token};
+      push @violations, {
+        file    => $file,
+        line    => $line_number,
+        text    => $line,
+        helper  => $rule->{helper},
+        message => $rule->{message},
+      };
+    }
+  }
+  close($fh);
+}
+
+is(scalar(@violations), 0, 'no raw vendor-specific SQL where a Bugzilla DB helper exists');
+
+if (@violations) {
+  diag(join("\n", map {
+    sprintf('%s:%d: %s (%s)', $_->{file}, $_->{line}, $_->{message}, $_->{text})
+  } @violations));
+}
+
+done_testing();