version 0.9.3. fixing circular dependency checker and ignoring not nu…

…ll primary keys that are also foreign keys which happens with schemas using MTI

README.md

@@ -72,7 +72,7 @@ Put this in your `test/spec_helper.rb`, `spec/spec_helper.rb`, or some other fil
 
 #### Stepford::FactoryGirl
 
-Stepford::FactoryGirl acts just like FactoryGirl, but it goes through all the null=false associations in the factory and/or its presence validated associations and attempts to create/build/build_stub depending on what you called originally, but also lets you pass in an `:with_factory_options` that can contain a hash of factory name symbols to the arguments and block you'd pass to it. You specify the block using a `:blk` option with a proc/lambda (probably a lambda) to use in that method.
+Stepford::FactoryGirl acts just like FactoryGirl, but it goes through all the null=false associations for foreign keys that aren't primary keys in the factory and/or its presence validated associations and attempts to create/build/build_stub depending on what you called originally, but also lets you pass in an `:with_factory_options` that can contain a hash of factory name symbols to the arguments and block you'd pass to it. You specify the block using a `:blk` option with a proc/lambda (probably a lambda) to use in that method.
 
 If you don't specify options, it's easy (note: it is even easier with the rspec helper- see below). If Foo requires Bar and Bar requires a list of Foobars and a Barfoo, and you have factories for each of those, you'd only have to do:
 
@@ -101,14 +101,7 @@ Then you can just use `create`, `create_list`, `build`, `build_list`, or `build_
 
 ##### Stopping Circular References
 
-If you have a circular reference (A has NOT NULL foreign key to B that has NOT NULL foreign key to C that has NOT NULL foreign key to A) in the
-schema, there is a workaround. First, prepopulate one of the models involved in the interdependency chain in the database as part of test setup,
-or if the ids are NOT NULL but are not foreign key constrained (i.e. if you can enter an invalid ID into the foreign key column, which implies possible 
-referential integrity issues) then you may be able to set them with an invalid id. Take that foreign id and then use the following to ensure
-that it will set that foreign id or instance. This is done at a global level which may not work for you, but it makes it convenient to put into
-your spec/spec_helper.rb, etc. For example, let's say your bar has NOT NULL on bartender_id and waiter_id, and in turn bartender and waiter
-both have a NOT NULL bar_id, and neither enforce foreign keys. Maybe you have preloaded data for waiter somehow as the id '123', but want to set bartender to
-just use an invalid id '-1', and you want to do it when they are on their second loop. You could use:
+If you have a circular reference (A has NOT NULL foreign key to B that has NOT NULL foreign key to C that has NOT NULL foreign key to A) either via schema where the foreign key is not also a primary key of the model with the belongs_to, or there is an ActiveRecord presence validation), there is a workaround. First, prepopulate one of the models involved in the interdependency chain in the database as part of test setup, or if the ids are NOT NULL but are not foreign key constrained (i.e. if you can enter an invalid ID into the foreign key column, which implies possible referential integrity issues) then you may be able to set them with an invalid id. Take that foreign id and then use the following to ensure that it will set that foreign id or instance. This is done at a global level which may not work for you, but it makes it convenient to put into your spec/spec_helper.rb, etc. For example, let's say your bar has NOT NULL on bartender_id and waiter_id, and in turn bartender and waiter both have a NOT NULL bar_id, and neither enforce foreign keys. Maybe you have preloaded data for waiter somehow as the id '123', but want to set bartender to just use an invalid id '-1', and you want to do it when they are on their second loop. You could use:
 
     Stepford::FactoryGirl.stop_circular_refs = {
        [:bartender, :bar] => {on_loop: 2, set_foreign_key_to: -1},
@@ -121,39 +114,35 @@ Stepford has a CLI with a circular reference checker and a generator to automati
 
 ##### Refs
 
-Check ActiveRecord circular dependencies:
+Check ActiveRecord circular dependencies where the foreign key for a belongs_to is not also a primary key of the model, or there is an ActiveRecord presence validation keeping an association from being null:
 
     bundle exec stepford circular
 
 Then it outputs the circular dependencies, e.g.:
 
     The following non-nullable foreign keys used in ActiveRecord model associations are involved in circular dependencies:
 
-    foo.bar_id -> bar.bartender_id -> bartender.sandwich_id -> sandwich.foo_id
+    beers.waitress_id -> waitresses.bartender_id -> bartenders.beer_id -> beers.waitress_id
 
-    foo.bar_id -> bar.waiter_id -> waiter.waitress_id
+    beers.waitress_id -> waitresses.bartender_id -> bartenders.order_id -> order.beer_id -> beers.waitress_id
 
-    waitress.waiter_id -> bar.waiter_id -> waiter.waitress_id
-
-    ...
 
     Distinct foreign keys involved in a circular dependency:
 
-    bar.bartender_id
-    bar.waiter_id
-    bartender.sandwich_id
-    foo.bar_id
-    sandwich.foo_id
-    waiter.waitress_id
-    waitress.waiter_id
+    beers.waitress_id
+    order.beer_id
+    bartenders.beer_id
+    bartenders.order_id
+    waitresses.bartender_id
+
 
     Foreign keys by number of circular dependency chains involved with:
 
-    3 (out of 6): bar.bartender_id -> bartender
-    2 (out of 6): bar.waiter_id -> waiter
-    1 (out of 6): bartender.sandwich_id -> sandwich
-    1 (out of 6): foo.bar_id -> bar
-    ...
+    2 (out of 2): beers.waitress_id -> waitresses
+    2 (out of 2): waitresses.bartender_id -> bartenders
+    1 (out of 2): order.beer_id -> beers
+    1 (out of 2): bartenders.order_id -> order
+    1 (out of 2): bartenders.beer_id -> beers
 
 ##### Factories
 

lib/stepford/circular_ref_checker.rb

@@ -1,8 +1,6 @@
 module Stepford
   class CircularRefChecker
 
-    @@model_and_association_names = []
-    @@level = 0
     @@offenders = []
     @@circles_sorted = []
     @@circles = []
@@ -26,6 +24,13 @@ def self.check_refs(options={})
         check_associations(model_class)
       end
 
+      if @@circles.size == 0
+        puts
+        puts "No circular dependencies."
+        puts
+        return true
+      end
+
       puts "The following non-nullable foreign keys used in ActiveRecord model associations are involved in circular dependencies:"
       @@circles.sort.each do |c|
         puts
@@ -55,43 +60,46 @@ def self.check_refs(options={})
         t = arr[1]
         puts "#{t} (out of #{@@circles_sorted.size}): #{c[0]}.#{c[1]} -> #{c[2]}"
       end
+      puts
 
-      return (@@offenders.size == 0)
+      return false
     end
 
-    def self.check_associations(model_class)
-      @@level += 1
-
+    def self.check_associations(model_class, model_and_association_names = [])
       model_class.reflections.collect {|association_name, reflection|
-        @@model_and_association_names = [] if @@level == 1
         next unless reflection.macro == :belongs_to
+        puts "warning: #{model_class}'s association #{reflection.name}'s foreign_key was nil. can't check." unless reflection.foreign_key
         assc_sym = reflection.name.to_sym
         clas_sym = reflection.class_name.underscore.to_sym
         next_class = clas_sym.to_s.camelize.constantize
 
         # if has a foreign key, then if NOT NULL or is a presence validate, the association is required and should be output. unfortunately this could mean a circular reference that will have to be manually fixed
         has_presence_validator = model_class.validators_on(assc_sym).collect{|v|v.class}.include?(ActiveModel::Validations::PresenceValidator)
-        required = reflection.foreign_key ? (has_presence_validator || model_class.columns.any?{|c| !c.null && c.name.to_sym == reflection.foreign_key.to_sym}) : false
-        if required
-          key = [model_class.table_name.to_sym, reflection.foreign_key.to_sym, next_class.table_name]
-          if @@model_and_association_names.include?(key)
-            @@offenders << @@model_and_association_names.last unless @@offenders.include?(@@model_and_association_names.last)
-            short = @@model_and_association_names.dup
+        # note: supports composite_primary_keys gem which stores primary_key as an array
+        foreign_key_is_also_primary_key = Array.wrap(model_class.primary_key).collect{|pk|pk.to_sym}.include?(reflection.foreign_key.to_sym)
+        is_not_null_fkey_that_is_not_primary_key = model_class.columns.any?{|c| !c.null && c.name.to_sym == reflection.foreign_key.to_sym && !foreign_key_is_also_primary_key}
+
+        if is_not_null_fkey_that_is_not_primary_key || has_presence_validator
+          key = [model_class.table_name.to_sym, reflection.foreign_key.to_sym, next_class.table_name.to_sym]
+          if model_and_association_names.include?(key)
+            @@offenders << model_and_association_names.last unless @@offenders.include?(model_and_association_names.last)
+            short = model_and_association_names.dup
             # drop all preceding keys that have nothing to do with the circle
             (short.index(key)).times {short.delete_at(0)}
             sorted = short.sort
             unless @@circles_sorted.include?(sorted)
               @@circles_sorted << sorted
-              @@circles << "#{(short << key).collect{|b|"#{b[0]}.#{b[1]}"}.join(' -> ')}".to_sym
+              @@circles << "#{(short + [key]).collect{|b|"#{b[0]}.#{b[1]}"}.join(' -> ')}".to_sym
             end
           else
-            @@model_and_association_names << key
-            check_associations(next_class)
+            model_and_association_names << key
+            check_associations(next_class, model_and_association_names)
           end
         end
       }
 
-      @@level -= 1
+      model_and_association_names.pop
+      model_and_association_names
     end
   end
 end

lib/stepford/factory_girl.rb

@@ -66,7 +66,13 @@ def method_missing(m, *args, &block)
             has_presence_validator = model_class.validators_on(assc_sym).collect{|v|v.class}.include?(::ActiveModel::Validations::PresenceValidator)
             required = reflection.foreign_key ? (has_presence_validator || model_class.columns.any?{|c| !c.null && c.name.to_sym == reflection.foreign_key.to_sym}) : false
             orig_method_args_and_options = with_factory_options ? (with_factory_options[[clas_sym, assc_sym]] || with_factory_options[clas_sym]) : nil
-            if required || orig_method_args_and_options
+            # if has a foreign key, then if NOT NULL or is a presence validate, the association is required and should be output. unfortunately this could mean a circular reference that will have to be manually fixed
+            has_presence_validator = model_class.validators_on(assc_sym).collect{|v|v.class}.include?(ActiveModel::Validations::PresenceValidator)
+            # note: supports composite_primary_keys gem which stores primary_key as an array
+            foreign_key_is_also_primary_key = Array.wrap(model_class.primary_key).collect{|pk|pk.to_sym}.include?(reflection.foreign_key.to_sym)
+            is_not_null_fkey_that_is_not_primary_key = model_class.columns.any?{|c| !c.null && c.name.to_sym == reflection.foreign_key.to_sym && !foreign_key_is_also_primary_key}
+
+            if is_not_null_fkey_that_is_not_primary_key || has_presence_validator
               circular_ref_key = [model_sym, assc_sym]
               all_opts = ::Stepford::FactoryGirl.stop_circular_refs
               if all_opts.is_a?(Hash) && all_opts.size > 0

lib/stepford/version.rb

@@ -1,3 +1,3 @@
 module Stepford
-  VERSION = '0.9.2'
+  VERSION = '0.9.3'
 end