diff --git a/spec/granite/connection_management_spec.cr b/spec/granite/connection_management_spec.cr index 995f73e1..06a6a5d9 100644 --- a/spec/granite/connection_management_spec.cr +++ b/spec/granite/connection_management_spec.cr @@ -5,10 +5,31 @@ describe "Granite::Base track time since last write" do ReplicatedChat.connection_switch_wait_period = 250 ReplicatedChat.new(content: "hello world!").save! sleep 500.milliseconds + Fiber.current.granite_adapters.try(&.clear) current_url = ReplicatedChat.adapter.url reader_connection = Granite::Connections["#{ENV["CURRENT_ADAPTER"]}_with_replica"] raise "Reader connection cannot be nil" if reader_connection.nil? reader_url = reader_connection[:reader].url current_url.should eq reader_url end + + it "should support isolated concurrent adapter routing across fibers" do + ReplicatedChat.switch_to_writer_adapter + # In the main fiber, ReplicatedChat.adapter should be the writer + ReplicatedChat.adapter.should eq ReplicatedChat.writer_adapter + + ch = Channel(Nil).new + + spawn do + # Switch to reader in this concurrent fiber + ReplicatedChat.switch_to_reader_adapter + ReplicatedChat.adapter.should eq ReplicatedChat.reader_adapter + ch.send(nil) + end + + ch.receive + + # Back in the main fiber, it should still be the writer adapter + ReplicatedChat.adapter.should eq ReplicatedChat.writer_adapter + end end diff --git a/spec/granite/validation_helpers/inequality_spec.cr b/spec/granite/validation_helpers/inequality_spec.cr index 5b845d4b..c68452ab 100644 --- a/spec/granite/validation_helpers/inequality_spec.cr +++ b/spec/granite/validation_helpers/inequality_spec.cr @@ -20,10 +20,8 @@ describe Granite::ValidationHelpers do less_than_test.errors[3].message.should eq "float_32_lte must be less than or equal to 100.25" less_than_test_nil = Validators::LessThanTest.new - - expect_raises(Exception, "Nil assertion failed") do - less_than_test_nil.save - end + less_than_test_nil.save + less_than_test_nil.errors.size.should eq 4 end end @@ -46,10 +44,8 @@ describe Granite::ValidationHelpers do greater_than_test.errors[3].message.should eq "float_32_lte must be greater than or equal to 100.25" greater_than_test = Validators::GreaterThanTest.new - - expect_raises(Exception, "Nil assertion failed") do - greater_than_test.save - end + greater_than_test.save + greater_than_test.errors.size.should eq 4 end end end diff --git a/spec/granite/validation_helpers/lenght_spec.cr b/spec/granite/validation_helpers/lenght_spec.cr index 75db1f8e..628cd53e 100644 --- a/spec/granite/validation_helpers/lenght_spec.cr +++ b/spec/granite/validation_helpers/lenght_spec.cr @@ -14,5 +14,17 @@ describe Granite::ValidationHelpers do length_test.errors[0].message.should eq "title is too short. It must be at least 5" length_test.errors[1].message.should eq "description is too long. It must be at most 25" end + + it "should handle nil values correctly for length validations" do + length_test = Validators::LengthTest.new + length_test.title = nil + length_test.description = nil + length_test.save + + # title being nil fails min length validation -> 1 error + # description being nil passes max length validation -> 0 errors + length_test.errors.size.should eq 1 + length_test.errors[0].message.should eq "title is too short. It must be at least 5" + end end end diff --git a/src/granite/columns.cr b/src/granite/columns.cr index 1dd65fff..79c9a50c 100644 --- a/src/granite/columns.cr +++ b/src/granite/columns.cr @@ -89,16 +89,20 @@ module Granite::Columns end def {{decl.var.id}}! : {{not_nilable_type}} - raise NilAssertionError.new {{@type.name.stringify}} + "#" + {{decl.var.stringify}} + " cannot be nil" if @{{decl.var}}.nil? - @{{decl.var}}.not_nil! + val = @{{decl.var}} + raise NilAssertionError.new {{@type.name.stringify}} + "#" + {{decl.var.stringify}} + " cannot be nil" if val.nil? + val end + {% else %} def {{decl.var.id}}=(@{{decl.var.id}} : {{type.id}}); end def {{decl.var.id}} : {{type.id}} - raise NilAssertionError.new {{@type.name.stringify}} + "#" + {{decl.var.stringify}} + " cannot be nil" if @{{decl.var}}.nil? - @{{decl.var}}.not_nil! + val = @{{decl.var}} + raise NilAssertionError.new {{@type.name.stringify}} + "#" + {{decl.var.stringify}} + " cannot be nil" if val.nil? + val end + {% end %} end diff --git a/src/granite/connection_management.cr b/src/granite/connection_management.cr index bd654ff0..6a450611 100644 --- a/src/granite/connection_management.cr +++ b/src/granite/connection_management.cr @@ -1,3 +1,9 @@ +require "atomic" + +class Fiber + property granite_adapters : Hash(String, Granite::Adapter::Base)? +end + module Granite::ConnectionManagement macro included # Default value for the time a model waits before using a reader @@ -5,36 +11,60 @@ module Granite::ConnectionManagement # all models use this value. Change it # to change it in all Granite::Base models. class_property connection_switch_wait_period : Int32 = Granite::Connections.connection_switch_wait_period - @@last_write_time = Time.instant - class_property current_adapter : Granite::Adapter::Base? + # Note: @@reader_adapter and @@writer_adapter remain unsynchronized class-level globals. + # This is safe assuming connection configurations are initialized at application startup only. class_property reader_adapter : Granite::Adapter::Base? class_property writer_adapter : Granite::Adapter::Base? - def self.last_write_time - @@last_write_time - end + {% if compare_versions(Crystal::VERSION, "1.20.0") >= 0 %} + @@epoch = Time.instant + @@last_write_time = Atomic(Int64).new(0_i64) - # This is done this way because callbacks don't work on class mthods - def self.update_last_write_time - @@last_write_time = Time.instant - end + def self.last_write_time : Time::Instant + @@epoch + @@last_write_time.get.milliseconds + end + + # This is done this way because callbacks don't work on class mthods + def self.update_last_write_time + @@last_write_time.set((Time.instant - @@epoch).total_milliseconds.to_i64) + end + + def self.time_since_last_write : Time::Span + Time.instant - last_write_time + end + {% else %} + @@epoch = Time.monotonic + @@last_write_time = Atomic(Int64).new(0_i64) + + def self.last_write_time : Time::Span + @@epoch + @@last_write_time.get.milliseconds + end + + # This is done this way because callbacks don't work on class mthods + def self.update_last_write_time + @@last_write_time.set((Time.monotonic - @@epoch).total_milliseconds.to_i64) + end + + def self.time_since_last_write : Time::Span + Time.monotonic - last_write_time + end + {% end %} def update_last_write_time self.class.update_last_write_time end - def self.time_since_last_write - Time.instant - @@last_write_time - end - def time_since_last_write self.class.time_since_last_write end def self.switch_to_reader_adapter if time_since_last_write > @@connection_switch_wait_period.milliseconds - @@current_adapter = @@reader_adapter + fiber_adapters = Fiber.current.granite_adapters ||= {} of String => Granite::Adapter::Base + if reader = @@reader_adapter + fiber_adapters[self.name] = reader + end end end @@ -43,7 +73,10 @@ module Granite::ConnectionManagement end def self.switch_to_writer_adapter - @@current_adapter = @@writer_adapter + fiber_adapters = Fiber.current.granite_adapters ||= {} of String => Granite::Adapter::Base + if writer = @@writer_adapter + fiber_adapters[self.name] = writer + end end def switch_to_writer_adapter @@ -53,12 +86,9 @@ module Granite::ConnectionManagement def self.schedule_adapter_switch return if @@writer_adapter == @@reader_adapter - spawn do - sleep @@connection_switch_wait_period.milliseconds - switch_to_reader_adapter - end - - Fiber.yield + # In M:N multithreading, spawning a fiber to mutate global state or Fiber local state + # is no longer safe or deterministic. We rely on the dynamic check in `adapter` method + # and the Fiber-local scope. end def schedule_adapter_switch @@ -66,10 +96,26 @@ module Granite::ConnectionManagement end def self.adapter - begin - @@current_adapter.not_nil! - rescue NilAssertionError - Granite::Connections.registered_connections.first?.not_nil![:writer] + fiber_adapters = Fiber.current.granite_adapters + + if fiber_adapters && (adapter = fiber_adapters[self.name]?) + return adapter + end + + if time_since_last_write > @@connection_switch_wait_period.milliseconds + if reader = @@reader_adapter + return reader + end + else + if writer = @@writer_adapter + return writer + end + end + + if first_conn = Granite::Connections.registered_connections.first? + first_conn[:writer] + else + raise "No registered connections found" end end end @@ -82,10 +128,12 @@ module Granite::ConnectionManagement #{Granite::Connections.registered_connections.map{ |conn| "#{conn[:writer].name}"}.join(", ")}" - raise error_message if Granite::Connections[{{name}}].nil? - - self.writer_adapter = Granite::Connections[{{name}}].not_nil![:writer] - self.reader_adapter = Granite::Connections[{{name}}].not_nil![:reader] - self.current_adapter = @@writer_adapter + if conn = Granite::Connections[{{name}}] + self.writer_adapter = conn[:writer] + self.reader_adapter = conn[:reader] + else + raise error_message + end + # self.current_adapter = @@writer_adapter end end diff --git a/src/granite/validation_helpers/inequality.cr b/src/granite/validation_helpers/inequality.cr index 08ab181a..45f10d5c 100644 --- a/src/granite/validation_helpers/inequality.cr +++ b/src/granite/validation_helpers/inequality.cr @@ -1,9 +1,9 @@ module Granite::ValidationHelpers macro validate_greater_than(field, amount, or_equal_to = false) - validate {{field}}, "#{{{field}}} must be greater than#{{{or_equal_to}} ? " or equal to" : ""} #{{{amount}}}", Proc(self, Bool).new { |model| (model.{{field.id}}.not_nil! {% if or_equal_to %} >= {% else %} > {% end %} {{amount.id}}) } + validate {{field}}, "#{{{field}}} must be greater than#{{{or_equal_to}} ? " or equal to" : ""} #{{{amount}}}", Proc(self, Bool).new { |model| ((val = model.{{field.id}}) ? (val {% if or_equal_to %} >= {% else %} > {% end %} {{amount.id}}) : false) } end macro validate_less_than(field, amount, or_equal_to = false) - validate {{field}}, "#{{{field}}} must be less than#{{{or_equal_to}} ? " or equal to" : ""} #{{{amount}}}", Proc(self, Bool).new { |model| (model.{{field.id}}.not_nil! {% if or_equal_to %} <= {% else %} < {% end %} {{amount.id}}) } + validate {{field}}, "#{{{field}}} must be less than#{{{or_equal_to}} ? " or equal to" : ""} #{{{amount}}}", Proc(self, Bool).new { |model| ((val = model.{{field.id}}) ? (val {% if or_equal_to %} <= {% else %} < {% end %} {{amount.id}}) : false) } end end diff --git a/src/granite/validation_helpers/length.cr b/src/granite/validation_helpers/length.cr index fac4eae8..5acee325 100644 --- a/src/granite/validation_helpers/length.cr +++ b/src/granite/validation_helpers/length.cr @@ -1,9 +1,9 @@ module Granite::ValidationHelpers macro validate_min_length(field, length) - validate {{field}}, "#{{{field}}} is too short. It must be at least #{{{length}}}", Proc(self, Bool).new { |model| (model.{{field.id}}.not_nil!.size >= {{length.id}}) } + validate {{field}}, "#{{{field}}} is too short. It must be at least #{{{length}}}", Proc(self, Bool).new { |model| ((val = model.{{field.id}}) ? val.size >= {{length.id}} : false) } end macro validate_max_length(field, length) - validate {{field}}, "#{{{field}}} is too long. It must be at most #{{{length}}}", Proc(self, Bool).new { |model| (model.{{field.id}}.not_nil!.size <= {{length.id}}) } + validate {{field}}, "#{{{field}}} is too long. It must be at most #{{{length}}}", Proc(self, Bool).new { |model| ((val = model.{{field.id}}) ? val.size <= {{length.id}} : true) } end end