From 516e36b764450da4e47e277ec59b2510e391c0d2 Mon Sep 17 00:00:00 2001
From: Andrew Gregory <andrew.gregory.8@gmail.com>
Date: Sun, 14 Dec 2014 17:29:45 -0500
Subject: [PATCH] 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.

Fixes #46.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com>
---
 lib/SQL/Translator/Producer/MySQL.pm  |  7 +++--
 lib/SQL/Translator/Producer/SQLite.pm |  4 ++-
 t/38-mysql-producer.t                 | 45 ++++++++++++++++++++++++++-
 t/56-sqlite-producer.t                | 32 +++++++++++++++++++
 4 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/lib/SQL/Translator/Producer/MySQL.pm b/lib/SQL/Translator/Producer/MySQL.pm
index 904b294c3..6d18f0eb2 100644
--- 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.*\sEND\s*$/i) {
+            $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
index 1c04edde3..71f1d1b21 100644
--- 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.*\sEND$/i;
     }
     else {
       $action = $trigger->action->{for_each} . " "
diff --git a/t/38-mysql-producer.t b/t/38-mysql-producer.t
index 446186bdf..b3c3bf88c 100644
--- 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
index 66bb8bb4c..24941818f 100644
--- 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;
@@ -192,4 +193,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;