Fix force_reload for loaded ActiveRecord transitions

The current implementation [1] for force reloading will use the
in-memory transitions if they're available instead of forcing a DB query
like you'd expect when passing this flag.

This means that `force_reload: true` currently forces a cache-level
reload (an instance variable holding the most recent state in memory)
of the most recent transition, but there is no way to force a DB-level
reload when the transitions are in memory.

In most cases the current behaviour works as expected – accessing
`model.state_machine.current_state` will issue a SELECT with LIMIT 1,
so the full set of the model's transitions will not be loaded into
memory and subsequent calls to `current_state` will issue fresh queries.
This breaks when you actually load the association, e.g. with
`model.history.inspect`. Once this has happened, the adapter's `history`
(and thus also methods such as `model.state_machine.current_state`) will
use the in-memory transitions whether `force_reload` is true or false,
and never issue fresh queries.

In practice, you often want a way to reload the transitions from the DB
to ensure that you pick up on any new transitions from other processes,
or other instances of the same model in the same process. This is what
is expected when you explicitly ask statesman to `force_reload`!

This change simply passes `force_reload` along to `history`, and checks
that it's false before going ahead and using in-memory transitions.

This does not affect the memory adapter, which naturally has the correct
state in memory, or the Mongoid adapter (verified with a new spec).

[1]: https://github.com/gocardless/statesman/blob/14250521d38449244e9bcb8f5b31b76e26614c39/lib/statesman/adapters/active_record.rb#L44-L60

lib/statesman/adapters/active_record.rb

@@ -41,8 +41,8 @@ def create(from, to, metadata = {})
         @last_transition = nil
       end
 
-      def history
-        if transitions_for_parent.loaded?
+      def history(force_reload: false)
+        if transitions_for_parent.loaded? && !force_reload
           # Workaround for Rails bug which causes infinite loop when sorting
           # already loaded result set. Introduced in rails/rails@b097ebe
           transitions_for_parent.to_a.sort_by(&:sort_key)
@@ -53,7 +53,7 @@ def history
 
       def last(force_reload: false)
         if force_reload
-          @last_transition = history.last
+          @last_transition = history(force_reload: true).last
         else
           @last_transition ||= history.last
         end

lib/statesman/adapters/memory.rb

@@ -4,7 +4,6 @@ module Statesman
   module Adapters
     class Memory
       attr_reader :transition_class
-      attr_reader :history
       attr_reader :parent_model
 
       # We only accept mode as a parameter to maintain a consistent interface
@@ -32,6 +31,10 @@ def last(*)
         @history.sort_by(&:sort_key).last
       end
 
+      def history(*)
+        @history
+      end
+
       private
 
       def next_sort_key

lib/statesman/adapters/mongoid.rb

@@ -32,7 +32,7 @@ def create(from, to, metadata = {})
         @last_transition = nil
       end
 
-      def history
+      def history(*)
         transitions_for_parent.asc(:sort_key)
       end
 

spec/statesman/adapters/active_record_spec.rb

@@ -234,17 +234,35 @@
           described_class.new(MyActiveRecordModelTransition, model, observer)
         end
 
-        before { alternate_adapter.create(:y, :z, []) }
-
         it "still returns the cached value" do
+          alternate_adapter.create(:y, :z, [])
+
           expect_any_instance_of(MyActiveRecordModel).
             to receive(:my_active_record_model_transitions).never
           expect(adapter.last.to_state).to eq("y")
         end
 
-        context "when explitly not using the cache" do
-          it "still returns the cached value" do
-            expect(adapter.last(force_reload: true).to_state).to eq("z")
+        context "when explicitly not using the cache" do
+          context "when the transitions are in memory" do
+            before do
+              model.my_active_record_model_transitions.load
+              alternate_adapter.create(:y, :z, [])
+            end
+
+            it "reloads the value" do
+              expect(adapter.last(force_reload: true).to_state).to eq("z")
+            end
+          end
+
+          context "when the transitions are not in memory" do
+            before do
+              model.my_active_record_model_transitions.reset
+              alternate_adapter.create(:y, :z, [])
+            end
+
+            it "reloads the value" do
+              expect(adapter.last(force_reload: true).to_state).to eq("z")
+            end
           end
         end
       end

spec/statesman/adapters/mongoid_spec.rb

@@ -48,5 +48,35 @@
         end
       end
     end
+
+    context "when a new transition has been created elsewhere" do
+      let(:alternate_adapter) do
+        described_class.new(MyMongoidModelTransition, model, observer)
+      end
+
+      context "when explicitly not using the cache" do
+        context "when the transitions are in memory" do
+          before do
+            model.my_mongoid_model_transitions.entries
+            alternate_adapter.create(:y, :z)
+          end
+
+          it "reloads the value" do
+            expect(adapter.last(force_reload: true).to_state).to eq("z")
+          end
+        end
+
+        context "when the transitions are not in memory" do
+          before do
+            model.my_mongoid_model_transitions.reset
+            alternate_adapter.create(:y, :z)
+          end
+
+          it "reloads the value" do
+            expect(adapter.last(force_reload: true).to_state).to eq("z")
+          end
+        end
+      end
+    end
   end
 end