Replace find_in_generation with a more generic method, in_generation,…

… which accepts a block to execute in the scope defined by the previous generation.

lib/fixmytransport/data_generations.rb

@@ -23,7 +23,7 @@ class << self
            default_scope :conditions => [ "#{quoted_table_name}.generation_low <= ?
                                            AND #{quoted_table_name}.generation_high >= ?",
                                            CURRENT_GENERATION, CURRENT_GENERATION ]
-           # This callback sets the data generations columns to the current generation 
+           # This callback sets the data generations columns to the current generation
            # if not value has been set on them
            before_create :set_generations
 
@@ -78,13 +78,13 @@ def normalize_slug_sequences(data_generation)
 
       end
 
-      # Set the scope of a find call to a specific generation
-      def find_in_generation(generation_id, *params)
+      # Perform a block of code in the context of the data generation passed
+      def in_generation(generation_id, &block)
         self.with_exclusive_scope do
           self.with_scope(:find => {:conditions => [ "#{quoted_table_name}.generation_low <= ?
                                                       AND #{quoted_table_name}.generation_high >= ?",
                                                       generation_id, generation_id ]}) do
-             find(*params)
+             yield
           end
         end
       end
@@ -93,7 +93,10 @@ def find_in_generation(generation_id, *params)
       # return that instance (if it is valid in the current generation), or its successor, if
       # it has one
       def find_successor(*find_params)
-        previous = self.find_in_generation(PREVIOUS_GENERATION, *find_params)
+        previous = nil
+        self.in_generation(PREVIOUS_GENERATION) do
+          previous = self.find(*find_params)
+        end
         if previous
           return previous if previous.generation_high >= CURRENT_GENERATION
           successor = self.find(:first, :conditions => ['previous_id = ?', previous.id])
@@ -178,10 +181,10 @@ def slug_sequence_in_previous_generation
             condition_string += " and scope = ?"
             params << self.slug.scope
           end
-          previous_slug = Slug.find_in_generation(PREVIOUS_GENERATION,
-                                                  :first,
-                                                  :conditions => [condition_string] + params)
-
+          previous_slug = nil
+          Slug.in_generation(PREVIOUS_GENERATION) do
+            previous_slug = Slug.find(:first, :conditions => [condition_string] + params)
+          end
           return previous_slug.sequence if previous_slug
         end
         # If it didn't have the same slug and scope, return an integer one bigger than the
@@ -193,10 +196,11 @@ def slug_sequence_in_previous_generation
           condition_string += " and scope = ?"
           params << self.slug.scope
         end
-        max_previous_slug = Slug.find_in_generation(PREVIOUS_GENERATION,
-                                                :first,
-                                                :conditions => [condition_string] + params,
+        max_previous_slug = nil
+        Slug.in_generation(PREVIOUS_GENERATION) do
+          max_previous_slug = Slug.find(:first, :conditions => [condition_string] + params,
                                                 :order => 'sequence desc')
+        end
         if max_previous_slug
           return max_previous_slug.sequence + 1
         else
@@ -209,7 +213,9 @@ def previous
           return self
         end
         if self.previous_id
-          return self.class.find_in_generation(PREVIOUS_GENERATION, previous_id)
+          self.class.in_generation(PREVIOUS_GENERATION) do
+            return self.class.find(previous_id)
+          end
         end
         return nil
       end

lib/tasks/data_loader.rb

@@ -246,9 +246,9 @@ def replay_updates(model_class, dryrun=true, verbose=false)
         if changes.first[:event] == 'create'
           # This was a locally created object, so find the model in the previous generation
           puts "#{model_name} created locally. Looking in previous generation."
-          existing = model_class.find_in_generation(PREVIOUS_GENERATION,
-                                                    :first,
-                                                    :conditions => identity_hash)
+          model_class.in_generation(PREVIOUS_GENERATION) do
+            existing = find(:first, :conditions => identity_hash)
+          end
           if ! existing
             puts ["Can't find locally created #{model_name} in previous generation to update",
                   "for #{identity_hash.inspect}"].join(" ") if verbose
@@ -494,7 +494,10 @@ def load_instances_in_generation(model_class, parser, &block)
         next
       end
       search_conditions = fields_to_attribute_hash(change_in_place_fields, instance)
-      existing = model_class.find_in_generation(previous_generation, :first, :conditions => search_conditions)
+      existing = nil
+      model_class.in_generation(previous_generation) do
+        existing = find(:first, :conditions => search_conditions)
+      end
       # make a string we can use in output to identify the new instance by it's key attributes
       if existing
         puts "Updating and setting generation_high to #{generation} on #{reference_string(model_class, existing, change_in_place_fields)}" if verbose
@@ -516,7 +519,9 @@ def load_instances_in_generation(model_class, parser, &block)
       else
         # Can we find this record at all in the previous generation?
         search_conditions = fields_to_attribute_hash(identity_fields, instance)
-        existing = model_class.find_in_generation(previous_generation, :first, :conditions => search_conditions)
+        model_class.in_generation(previous_generation) do
+          existing = find(:first, :conditions => search_conditions)
+        end
         if existing
           puts "New record in this generation for existing instance #{reference_string(model_class, existing, change_in_place_fields)}" if verbose
           diff_hash = existing.diff(instance)

spec/models/operator_spec.rb

@@ -61,9 +61,10 @@
 
     it 'should be valid if it has a noc code already used in a previous generation' do
       noc_code = 'TRAI'
-      previous_operator = Operator.find_in_generation(PREVIOUS_GENERATION,
-                                                       :first,
-                                                       :conditions => ['noc_code = ?', noc_code])
+      previous_operator = nil
+      Operator.in_generation(PREVIOUS_GENERATION) do
+        previous_operator = Operator.find(:first, :conditions => ['noc_code = ?', noc_code])
+      end
       previous_operator.should_not == nil
       @operator.noc_code = previous_operator.noc_code
       @operator.valid?.should == true

spec/models/stop_area_spec.rb

@@ -87,9 +87,10 @@
 
     it 'should be valid if it has a code already used in a previous generation' do
       code = '910GVICTRIP'
-      previous_stop_area = StopArea.find_in_generation(PREVIOUS_GENERATION,
-                                                       :first,
-                                                       :conditions => ['code = ?', code])
+      previous_stop_area = nil
+      StopArea.in_generation(PREVIOUS_GENERATION) do
+        previous_stop_area = StopArea.find(:first, :conditions => ['code = ?', code])
+      end
       previous_stop_area.should_not == nil
       @stop_area.code = previous_stop_area.code
       @stop_area.valid?.should == true

spec/models/stop_spec.rb

@@ -110,9 +110,10 @@
 
       it 'should be valid if it has an atco code already used in a previous generation' do
         atco_code = '9100VICTRIP'
-        previous_stop = Stop.find_in_generation(PREVIOUS_GENERATION,
-                                                :first,
-                                                :conditions => ['atco_code = ?', atco_code])
+        previous_stop = nil
+        Stop.in_generation(PREVIOUS_GENERATION) do
+          previous_stop = Stop.find(:first, :conditions => ['atco_code = ?', atco_code])
+        end
         previous_stop.should_not == nil
         @stop.atco_code = previous_stop.atco_code
         @stop.valid?.should == true

spec/shared/controller_helpers.rb

@@ -8,97 +8,31 @@ module ControllerHelpers
 
         before do
           # stub the model finding query
-          find_params = [@default_params[:id]]
+          @find_params = [@default_params[:id]]
           if @scope_model
-            find_params << { :scope => @default_params[:scope], :include => [@scope_field] }
+            @find_params << { :scope => @default_params[:scope], :include => [@scope_field] }
           end
 
-          @model_type.stub!(:find).with(*find_params).and_raise(ActiveRecord::RecordNotFound)
-          if @scope_model
-            @previous_scope = mock_model(@scope_model)
-            @current_scope = mock_model(@scope_model)
-          end
-
-          # stub the previous generation query
-          @previous = mock_model(@model_type, :generation_high => PREVIOUS_GENERATION)
-          if @scope_model
-            @previous.stub!(@scope_field).and_return(@previous_scope)
-          end
-          @model_type.stub!(:find_in_generation).and_return(@previous)
-
+          @model_type.stub!(:find).with(*@find_params).and_raise(ActiveRecord::RecordNotFound)
           # stub the successor query
           @successor = mock_model(@model_type)
+
           if @scope_model
+            @current_scope = mock_model(@scope_model)
             @successor.stub!(@scope_field).and_return(@current_scope)
           end
-          @model_type.stub!(:find).with(:first, :conditions => ['previous_id = ?', @previous.id]).and_return(@successor)
+          @model_type.stub!(:find_successor).with(*@find_params).and_return(@successor)
         end
 
-        it 'should look for the instance in a previous generation' do
-          expected_args = [PREVIOUS_GENERATION, @default_params[:id]]
-          if @scope_model
-            expected_args << { :scope => @default_params[:scope],
-                               :include => [@scope_field] }
-          end
-          @model_type.should_receive(:find_in_generation).with(*expected_args)
+        it "should look for the instance's successor" do
+          @model_type.should_receive(:find_successor).with(*@find_params).and_return(@successor)
           make_request
         end
 
-        describe 'if the instance can be found in a previous generation' do
-
-          it 'should look for the successor to the instance in this generation' do
-            @model_type.should_receive(:find).with(:first, :conditions => ['previous_id = ?', @previous.id]).and_return(@successor)
-            make_request
-          end
-
-          describe 'if the instance is valid in this generation' do
-
-            before do
-              @previous.stub!(:generation_high).and_return(CURRENT_GENERATION)
-            end
-            it 'should issue a permanent redirect to the current friendly id of the stop' do
-              make_request
-              expected_params = { :id => @previous }
-              if @scope_model
-                expected_params[:scope] = @previous_scope
-              end
-              response.should redirect_to(@default_params.merge(expected_params))
-            end
-
-          end
-
-          describe 'if the instance is not valid in this generation' do
-
-            describe 'if there is a successor' do
-
-              it 'should issue a permanent redirect to the successor' do
-                make_request
-                expected_params = { :id => @successor }
-                if @scope_model
-                  expected_params[:scope] = @current_scope
-                end
-                response.should redirect_to(@default_params.merge(expected_params))
-              end
-
-            end
-
-            describe 'if there is no successor' do
-
-              before do
-                @model_type.stub!(:find).with(:first, :conditions => ['previous_id = ?', @previous.id]).and_return(nil)
-              end
-
-              it 'should re-raise the error (returning a 404 in production)' do
-                lambda{ make_request }.should raise_error(ActiveRecord::RecordNotFound)
-              end
-            end
-          end
-        end
-
-        describe 'if the instance cannot be found in a previous generation' do
+        describe 'if the successor cannot be found' do
 
           before do
-            @model_type.stub!(:find_in_generation).and_return(nil)
+            @model_type.stub!(:find_successor).and_return(nil)
           end
 
           it 'should re-raise the error (returning a 404 in production)' do

spec/shared/data_generation_helper.rb

@@ -77,19 +77,96 @@ module DataGenerationHelper
 
         it 'should find a model in the generation' do
           old_instance = create_model(generation_low=PREVIOUS_GENERATION, generation_high=PREVIOUS_GENERATION, @model_type, @default_attrs)
-          @model_type.find_in_generation(PREVIOUS_GENERATION, old_instance.id).should == old_instance
+          @model_type.in_generation(PREVIOUS_GENERATION){ @model_type.find(old_instance.id) }.should == old_instance
           old_instance.destroy
         end
 
         it 'should not find a model in the current generation' do
           current_instance = create_model(generation_low=CURRENT_GENERATION, generation_high=CURRENT_GENERATION, @model_type, @default_attrs)
           expected_error = "Couldn't find #{@model_type} with ID=#{current_instance.id}"
-          lambda{ @model_type.find_in_generation(PREVIOUS_GENERATION, current_instance.id) }.should raise_error(expected_error)
+          lambda{ @model_type.in_generation(PREVIOUS_GENERATION){ @model_type.find(current_instance.id) } }.should raise_error(expected_error)
           current_instance.destroy
         end
 
       end
 
+      describe "when finding the successor to a set of find parameters" do
+
+        before do
+          @find_params = [55]
+          if @scope_model
+            @find_params << { :scope => @default_params[:scope], :include => [@scope_field] }
+          end
+        end
+
+        it 'should look for an instance matching the find parameters in the previous generation' do
+          @model_type.should_receive(:in_generation).with(PREVIOUS_GENERATION).and_yield
+          # not really testing that this is in the scope of the previous generation
+          @model_type.should_receive(:find).with(*@find_params).and_return(@previous)
+          @model_type.find_successor(*@find_params)
+        end
+
+        describe 'if an instance can be found in a previous generation' do
+
+          before do
+            @previous = mock_model(@model_type, :generation_high => PREVIOUS_GENERATION,
+                                                :generation_low => PREVIOUS_GENERATION)
+            # this stubbed call is called in the scope of the previous generation
+            @model_type.stub(:find).with(*@find_params).and_return(@previous)
+          end
+
+          it 'should look for the successor to the instance in this generation' do
+            @model_type.should_receive(:find).with(:first, :conditions => ['previous_id = ?', @previous.id]).and_return(@successor)
+            @model_type.find_successor(*@find_params)
+          end
+
+          describe 'if the instance is valid in this generation' do
+
+            before do
+              @previous.stub!(:generation_high).and_return(CURRENT_GENERATION)
+            end
+
+            it 'should return the instance' do
+              @model_type.find_successor(*@find_params).should == @previous
+            end
+
+          end
+
+          describe 'if the instance is not valid in this generation' do
+
+            before do
+              @previous.stub!(:generation_high).and_return(PREVIOUS_GENERATION)
+              @successor = mock_model(@model_type)
+              @previous_conditions = { :conditions => ['previous_id = ?', @previous.id] }
+            end
+
+            describe 'if there is a successor' do
+
+              before do
+                @model_type.stub!(:find).with(:first, @previous_conditions).and_return(@successor)
+              end
+
+              it 'should return the successor' do
+                @model_type.find_successor(*@find_params).should == @successor
+              end
+
+            end
+
+            describe 'if there is no successor' do
+
+              before do
+                @model_type.stub!(:find).with(:first, @previous_conditions).and_return(nil)
+              end
+
+              it 'should return nil' do
+                @model_type.find_successor(*@find_params).should == nil
+              end
+            end
+          end
+        end
+
+      end
+
       describe 'when finding a model' do
 
         it 'should find a model in the current generation' do
@@ -112,27 +189,27 @@ module DataGenerationHelper
         end
 
       end
-      
-      describe 'when setting generations' do 
-        
-        it 'should not change existing generation attribute values' do 
+
+      describe 'when setting generations' do
+
+        it 'should not change existing generation attribute values' do
           instance = @model_type.new
           instance.generation_low = PREVIOUS_GENERATION
           instance.generation_high = PREVIOUS_GENERATION
           instance.should_not_receive(:generation_low=)
           instance.should_not_receive(:generation_high=)
           instance.set_generations
         end
-        
-        it 'should set nil generation attributes to the current generation' do 
+
+        it 'should set nil generation attributes to the current generation' do
           instance = @model_type.new
           instance.should_receive(:generation_low=).with(CURRENT_GENERATION)
           instance.should_receive(:generation_high=).with(CURRENT_GENERATION)
           instance.set_generations
         end
-        
+
       end
-      
+
     end
 
     shared_examples_for "a model that exists in data generations and has slugs" do