From eca1c1579ead61bb5976af821beb44082e405a6b Mon Sep 17 00:00:00 2001 From: Tatsujin Chin Date: Thu, 26 Apr 2018 08:06:40 +0800 Subject: [PATCH] Some enhancements about callbacks (#183) * add abort! * allow multiple callbacks * support block as callback * set new_record and destroyed properly remove duplicated methods in transactions.cr * add abort spec --- spec/granite_orm/callbacks/abort_spec.cr | 133 +++++++++++++++++++++++ spec/spec_models.cr | 27 +++++ src/granite_orm/callbacks.cr | 27 ++++- src/granite_orm/transactions.cr | 19 +--- 4 files changed, 188 insertions(+), 18 deletions(-) create mode 100644 spec/granite_orm/callbacks/abort_spec.cr diff --git a/spec/granite_orm/callbacks/abort_spec.cr b/spec/granite_orm/callbacks/abort_spec.cr new file mode 100644 index 00000000..13983a75 --- /dev/null +++ b/spec/granite_orm/callbacks/abort_spec.cr @@ -0,0 +1,133 @@ +require "../../spec_helper" + +{% for adapter in GraniteExample::ADAPTERS %} +module {{adapter.capitalize.id}} + describe "{{ adapter.id }} #abort!" do + context "when create" do + it "doesn't run other callbacks if abort at before_save" do + cwa = CallbackWithAbort.new(abort_at: "before_save", do_abort: true) + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at before_save."]) + cwa.history.to_s.strip.should eq("") + CallbackWithAbort.find("before_save").should be_nil + end + + it "only runs before_save if abort at before_create" do + cwa = CallbackWithAbort.new(abort_at: "before_create", do_abort: true) + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at before_create."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + RUNS + CallbackWithAbort.find("before_create").should be_nil + end + + it "runs before_save, before_create and save successfully if abort at after_create" do + cwa = CallbackWithAbort.new(abort_at: "after_create", do_abort: true) + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at after_create."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + before_create + RUNS + CallbackWithAbort.find("after_create").should be_a(CallbackWithAbort) + end + + it "runs before_save, before_create, after_create and save successfully if abort at after_save" do + cwa = CallbackWithAbort.new(abort_at: "after_save", do_abort: true) + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at after_save."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + before_create + after_create + RUNS + CallbackWithAbort.find("after_save").should be_a(CallbackWithAbort) + end + end + + context "when update" do + it "doesn't run other callbacks if abort at before_save" do + CallbackWithAbort.new(abort_at: "before_save", do_abort: false).save + cwa = CallbackWithAbort.find!("before_save") + cwa.do_abort = true + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at before_save."]) + cwa.history.to_s.strip.should eq("") + CallbackWithAbort.find!("before_save").do_abort.should be_false + end + + it "only runs before_save if abort at before_update" do + CallbackWithAbort.new(abort_at: "before_update", do_abort: false).save + cwa = CallbackWithAbort.find!("before_update") + cwa.do_abort = true + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at before_update."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + RUNS + CallbackWithAbort.find!("before_update").do_abort.should be_false + end + + it "runs before_save, before_update and save successfully if abort at after_update" do + CallbackWithAbort.new(abort_at: "after_update", do_abort: false).save + cwa = CallbackWithAbort.find!("after_update") + cwa.do_abort = true + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at after_update."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + before_update + RUNS + CallbackWithAbort.find!("after_update").do_abort.should be_true + end + + it "runs before_save, before_update, after_update and save successfully if abort at after_save" do + CallbackWithAbort.new(abort_at: "after_save", do_abort: false).save + cwa = CallbackWithAbort.find!("after_save") + cwa.do_abort = true + cwa.save + + cwa.errors.map(&.to_s).should eq(["Aborted at after_save."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_save + before_update + after_update + RUNS + CallbackWithAbort.find!("after_save").do_abort.should be_true + end + end + + context "when destroy" do + it "doesn't run other callbacks if abort at before_destroy" do + CallbackWithAbort.new(abort_at: "before_destroy", do_abort: true).save + cwa = CallbackWithAbort.find!("before_destroy") + cwa.destroy + + cwa.errors.map(&.to_s).should eq(["Aborted at before_destroy."]) + cwa.history.to_s.strip.should eq("") + CallbackWithAbort.find("before_destroy").should be_a(CallbackWithAbort) + end + + it "runs before_destroy and destroy successfully if abort at after_destory" do + CallbackWithAbort.new(abort_at: "after_destroy", do_abort: true).save + cwa = CallbackWithAbort.find!("after_destroy") + cwa.destroy + + cwa.errors.map(&.to_s).should eq(["Aborted at after_destroy."]) + cwa.history.to_s.strip.should eq <<-RUNS + before_destroy + RUNS + CallbackWithAbort.find("after_destroy").should be_nil + end + end + end +end +{% end %} diff --git a/spec/spec_models.cr b/spec/spec_models.cr index 01de3cf9..05f8d6fb 100644 --- a/spec/spec_models.cr +++ b/spec/spec_models.cr @@ -267,6 +267,32 @@ end end end + class CallbackWithAbort < Granite::ORM::Base + adapter {{ adapter_literal }} + table_name callbacks_with_abort + primary abort_at : String, auto: false + field do_abort : Bool + + property history : IO::Memory = IO::Memory.new + + {% for name in Granite::ORM::Callbacks::CALLBACK_NAMES %} + {{name.id}} do + abort! if do_abort && abort_at == "{{name.id}}" + history << "{{name.id}}\n" + end + {% end %} + + def self.drop_and_create + exec "DROP TABLE IF EXISTS #{ quoted_table_name }" + exec <<-SQL + CREATE TABLE #{ quoted_table_name } ( + abort_at VARCHAR(31), + do_abort BOOL + ) + SQL + end + end + class Kvs < Granite::ORM::Base adapter {{ adapter_literal }} table_name kvss @@ -334,6 +360,7 @@ end Empty.drop_and_create ReservedWord.drop_and_create Callback.drop_and_create + CallbackWithAbort.drop_and_create Kvs.drop_and_create Book.drop_and_create BookReview.drop_and_create diff --git a/src/granite_orm/callbacks.cr b/src/granite_orm/callbacks.cr index f160625c..5a1a8eb5 100644 --- a/src/granite_orm/callbacks.cr +++ b/src/granite_orm/callbacks.cr @@ -1,6 +1,11 @@ module Granite::ORM::Callbacks + class Abort < Exception + end + CALLBACK_NAMES = %i(before_save after_save before_create after_create before_update after_update before_destroy after_destroy) + @_current_callback : Symbol? + macro included macro inherited CALLBACKS = { @@ -12,14 +17,30 @@ module Granite::ORM::Callbacks end {% for name in CALLBACK_NAMES %} - macro {{name.id}}(callback) - \{% CALLBACKS[{{name}}] << callback.id %} + macro {{name.id}}(*callbacks, &block) + \{% for callback in callbacks %} + \{% CALLBACKS[{{name}}] << callback %} + \{% end %} + \{% if block.is_a? Block %} + \{% CALLBACKS[{{name}}] << block %} + \{% end %} end macro __run_{{name.id}} + @_current_callback = {{name}} \{% for callback in CALLBACKS[{{name}}] %} - \{{callback}} + \{% if callback.is_a? Block %} + begin + \{{callback.body}} + end + \{% else %} + \{{callback.id}} + \{% end %} \{% end %} end {% end %} + + def abort!(message = "Aborted at #{@_current_callback}.") + raise Abort.new(message) + end end diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index 60a78b37..bd3e43a6 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -59,12 +59,12 @@ module Granite::ORM::Transactions rescue err raise DB::Error.new(err.message) end + @new_record = false __run_after_create end - @new_record = false __run_after_save return true - rescue ex : DB::Error + rescue ex : DB::Error | Granite::ORM::Callbacks::Abort if message = ex.message Granite::ORM.settings.logger.error "Save Exception: #{message}" errors << Granite::ORM::Error.new(:base, message) @@ -78,10 +78,10 @@ module Granite::ORM::Transactions begin __run_before_destroy @@adapter.delete(@@table_name, @@primary_name, {{primary_name}}) - __run_after_destroy @destroyed = true + __run_after_destroy return true - rescue ex : DB::Error + rescue ex : DB::Error | Granite::ORM::Callbacks::Abort if message = ex.message Granite::ORM.settings.logger.error "Destroy Exception: #{message}" errors << Granite::ORM::Error.new(:base, message) @@ -114,15 +114,4 @@ module Granite::ORM::Transactions def persisted? !(new_record? || destroyed?) end - - # Returns true if this object hasn't been saved yet. - getter? new_record : Bool = true - - # Returns true if this object has been destroyed. - getter? destroyed : Bool = false - - # Returns true if the record is persisted. - def persisted? - !(new_record? || destroyed?) - end end