Improve add_trigger consistency between producers

Update Producer::SQLite and Producer::MySQL to only wrap the trigger action in "BEGIN...END" when the user has not already done so, bringing them in line with other producers and the add_trigger documentation.
dbsrgits · Jun 24, 2015 · e533bcd · e533bcd
1 parent 237e485
commit e533bcd
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 4 deletions.
diff --git a/Changes b/Changes
@@ -2,6 +2,7 @@ Changes for SQL::Translator
 
  * Add support for monotonically increasing SQLite autoincs (GH#47)
  * Fix forgotten quoting in the MySQL DROP TABLE diff producer (GH#50)
+ * Improve add_trigger consistency between producers (GH#48)
  * Declare dependencies in deterministic order (RT#102859)
  * Multiple speedups of naive internal debugging mechanism (GH#54)
  * Remove dependency on List::MoreUtils ( http://is.gd/lmu_cac_debacle )

diff --git a/lib/SQL/Translator/Producer/MySQL.pm b/lib/SQL/Translator/Producer/MySQL.pm
@@ -356,11 +356,14 @@ sub create_trigger {
         }
 
         my $action = $trigger->action;
-        $action .= ";" unless $action =~ /;\s*\z/;
+        if($action !~ /^ \s* BEGIN [\s\;] .*? [\s\;] END [\s\;]* $/six) {
+            $action .= ";" unless $action =~ /;\s*\z/;
+            $action = "BEGIN $action END";
+        }
 
         push @statements, "DROP TRIGGER IF EXISTS " . $generator->quote($name) if $options->{add_drop_trigger};
         push @statements, sprintf(
-            "CREATE TRIGGER %s %s %s ON %s\n  FOR EACH ROW BEGIN %s END",
+            "CREATE TRIGGER %s %s %s ON %s\n  FOR EACH ROW %s",
             $generator->quote($name), $trigger->perform_action_when, $event,
             $generator->quote($trigger->on_table), $action,
         );

diff --git a/lib/SQL/Translator/Producer/SQLite.pm b/lib/SQL/Translator/Producer/SQLite.pm
@@ -341,7 +341,9 @@ sub create_trigger {
     $DB::single = 1;
     my $action = "";
     if (not ref $trigger->action) {
-      $action .= "BEGIN " . $trigger->action . " END";
+      $action = $trigger->action;
+      $action = "BEGIN " . $action . " END"
+        unless $action =~ /^ \s* BEGIN [\s\;] .*? [\s\;] END [\s\;]* $/six;
     }
     else {
       $action = $trigger->action->{for_each} . " "

diff --git a/t/38-mysql-producer.t b/t/38-mysql-producer.t
@@ -19,7 +19,7 @@ use FindBin qw/$Bin/;
 #=============================================================================
 
 BEGIN {
-    maybe_plan(73,
+    maybe_plan(75,
         'YAML',
         'SQL::Translator::Producer::MySQL',
         'Test::Differences',
@@ -800,3 +800,46 @@ EOV
     is(SQL::Translator::Producer::MySQL::alter_drop_constraint($constraint,$options),
        'ALTER TABLE `table` DROP PRIMARY KEY','valid drop primary key');
 }
+
+{
+    my $schema = SQL::Translator::Schema->new();
+    my $table = $schema->add_table( name => 'foo', fields => ['bar'] );
+
+    {
+        my $trigger = $schema->add_trigger(
+            name                => 'mytrigger',
+            perform_action_when => 'before',
+            database_events     => 'update',
+            on_table            => 'foo',
+            fields              => ['bar'],
+            action              => 'BEGIN baz(); END'
+        );
+        my ($def) = SQL::Translator::Producer::MySQL::create_trigger($trigger);
+        my $expected
+          = "--\n"
+          . "-- Trigger mytrigger\n"
+          . "--\n"
+          . "CREATE TRIGGER mytrigger before update ON foo\n"
+          . "  FOR EACH ROW BEGIN baz(); END";
+        is($def, $expected, 'trigger created');
+    }
+
+    {
+        my $trigger = $schema->add_trigger(
+            name                => 'mytrigger2',
+            perform_action_when => 'after',
+            database_events     => ['insert'],
+            on_table            => 'foo',
+            fields              => ['bar'],
+            action              => 'baz()'
+        );
+        my ($def) = SQL::Translator::Producer::MySQL::create_trigger($trigger);
+        my $expected
+          = "--\n"
+          . "-- Trigger mytrigger2\n"
+          . "--\n"
+          . "CREATE TRIGGER mytrigger2 after insert ON foo\n"
+          . "  FOR EACH ROW BEGIN baz(); END";
+        is($def, $expected, 'trigger created');
+    }
+}
diff --git a/t/56-sqlite-producer.t b/t/56-sqlite-producer.t
@@ -5,6 +5,7 @@ use strict;
 use Test::More;
 use Test::SQL::Translator qw(maybe_plan);
 
+use SQL::Translator::Schema;
 use SQL::Translator::Schema::View;
 use SQL::Translator::Schema::Table;
 use SQL::Translator::Producer::SQLite;
@@ -211,4 +212,35 @@ $SQL::Translator::Producer::SQLite::NO_QUOTES = 0;
     }
 }
 
+{
+    my $schema = SQL::Translator::Schema->new();
+    my $table = $schema->add_table( name => 'foo', fields => ['bar'] );
+
+    {
+        my $trigger = $schema->add_trigger(
+            name                => 'mytrigger',
+            perform_action_when => 'before',
+            database_events     => 'update',
+            on_table            => 'foo',
+            fields              => ['bar'],
+            action              => 'BEGIN baz() END'
+        );
+        my ($def) = SQL::Translator::Producer::SQLite::create_trigger($trigger);
+        is($def, 'CREATE TRIGGER "mytrigger" before update on "foo" BEGIN baz() END', 'trigger created');
+    }
+
+    {
+        my $trigger = $schema->add_trigger(
+            name                => 'mytrigger2',
+            perform_action_when => 'after',
+            database_events     => ['insert'],
+            on_table            => 'foo',
+            fields              => ['bar'],
+            action              => 'baz()'
+        );
+        my ($def) = SQL::Translator::Producer::SQLite::create_trigger($trigger);
+        is($def, 'CREATE TRIGGER "mytrigger2" after insert on "foo" BEGIN baz() END', 'trigger created');
+    }
+}
+
 done_testing;