v5.0.0: find_by_path and find_or_create_by_path` now takes either an array of strings or an array of attribute hashes

Author: mceachen

CHANGELOG.md

@@ -1,33 +1,48 @@
 # Changelog
 
+### 5.0.0
+
+#### Breaking API changes
+
+* `find_by_path` and `find_or_create_by_path` now takes either an array of strings 
+  or an array of attribute hashes, which can include the inheritance column for STI support.
+* Removed the extraneous `base_class` `acts_as_tree` option—it needlessly duplicated ActiveRecord's method.
+* Removed the unused `name` `acts_as_tree` option.
+
+#### Improvements and bugfixes
+
+* Cleaned up the inheritance support methods to delegate correctly to ActiveRecord
+* Fixed a query generation error when ancestor paths exceeded 50 items.
+* Documented the `.touch` option
+
 ### 4.6.3
 
- * More goodness from [Abdelkader Boudih](https://github.com/seuros), including rspec 3 support.
+* More goodness from [Abdelkader Boudih](https://github.com/seuros), including rspec 3 support.
 
 ### 4.6.2
 
- * Pulled in [106](https://github.com/mceachen/closure_tree/pull/106) which fixed a bug introduced
-   in 4.6.0 which broke if the numeric ordering column wasn't named 'sort_order'. Tests have been
-   added. Thanks for the fix, [Fission Xuiptz](https://github.com/fissionxuiptz)!
+* Pulled in [106](https://github.com/mceachen/closure_tree/pull/106) which fixed a bug introduced
+  in 4.6.0 which broke if the numeric ordering column wasn't named 'sort_order'. Tests have been
+  added. Thanks for the fix, [Fission Xuiptz](https://github.com/fissionxuiptz)!
 
 ### 4.6.1
 
- * Address [issue 60](https://github.com/mceachen/closure_tree/issues/60) (use `.empty?` rather 
-   than `.nil?`—thanks for the suggestion, [Leonel Galán](https://github.com/leonelgalan),
-   [Doug Mayer](https://github.com/doxavore) and [Samnang Chhun](https://github.com/samnang)!
+* Address [issue 60](https://github.com/mceachen/closure_tree/issues/60) (use `.empty?` rather 
+  than `.nil?`—thanks for the suggestion, [Leonel Galán](https://github.com/leonelgalan),
+  [Doug Mayer](https://github.com/doxavore) and [Samnang Chhun](https://github.com/samnang)!
 
 ### 4.6.0
 
- * Deterministically ordered trees are guaranteed to have a sort_order now.
+* Deterministically ordered trees are guaranteed to have a sort_order now.
+
+  **This may be a breaking change if you're expecting sort_order to be nullable.**
 
-   **This may be a breaking change if you're expecting sort_order to be nullable.**
+  Many thanks to [David Schmidt](https://github.com/inetdavid) for raising and
+  working on the issue!
 
-   Many thanks to [David Schmidt](https://github.com/inetdavid) for raising and
-   working on the issue!
+* Added ```append_child``` and ```prepend_child```
 
- * Added ```append_child``` and ```prepend_child```
- 
- * All raw SQL is now ```strip_heredoc```'ed
+* All raw SQL is now ```strip_heredoc```'ed
 
 ### 4.5.0
 

Gemfile

@@ -1,16 +1,21 @@
-source 'https://rubygems.org'
-gem 'foreigner', :git => 'https://github.com/mceachen/foreigner.git'
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "foreigner", :git => "https://github.com/mceachen/foreigner.git"
+gem "activerecord", "~> 3.2"
+gem "strong_parameters"
 
 platforms :ruby, :rbx do
-  gem 'mysql2'
-  gem 'pg'
-  gem 'sqlite3'
+  gem "mysql2"
+  gem "pg"
+  gem "sqlite3"
 end
 
 platforms :jruby do
-  gem 'activerecord-jdbcmysql-adapter'
-  gem 'activerecord-jdbcpostgresql-adapter'
-  gem 'activerecord-jdbcsqlite3-adapter'
+  gem "activerecord-jdbcmysql-adapter"
+  gem "activerecord-jdbcpostgresql-adapter"
+  gem "activerecord-jdbcsqlite3-adapter"
 end
 
 gemspec

README.md

@@ -10,7 +10,7 @@ and tracking user referrals.
 [![Code Climate](https://codeclimate.com/github/mceachen/closure_tree.png)](https://codeclimate.com/github/mceachen/closure_tree)
 [![Dependency Status](https://gemnasium.com/mceachen/closure_tree.png)](https://gemnasium.com/mceachen/closure_tree)
 
-Substantially more efficient than
+Dramatically more performant than
 [ancestry](https://github.com/stefankroes/ancestry) and
 [acts_as_tree](https://github.com/amerine/acts_as_tree), and even more
 awesome than [awesome_nested_set](https://github.com/collectiveidea/awesome_nested_set/),
@@ -25,16 +25,16 @@ closure_tree has some great features:
 * __Best-in-class mutation performance__:
   * 2 SQL INSERTs on node creation
   * 3 SQL INSERT/UPDATEs on node reparenting
+* __Support for [concurrency](#concurrency)__ (using [with_advisory_lock](https://github.com/mceachen/with_advisory_lock))
 * __Support for Rails 3.2, 4.0, and 4.1__
-* __Support for Ruby 1.9 and 2.1 (jRuby and Rubinius are still in development)__
+* __Support for Ruby 1.9, 2.1, and jRuby 1.6.13
 * Support for reparenting children (and all their descendants)
-* Support for [concurrency](#concurrency) (using [with_advisory_lock](https://github.com/mceachen/with_advisory_lock))
-* Support for polymorphism [STI](#sti) within the hierarchy
-* ```find_or_create_by_path``` for [building out hierarchies quickly and conveniently](#find_or_create_by_path)
-* Support for [deterministic ordering](#deterministic-ordering) of children
+* Support for [single-table inheritance (STI)](#sti) within the hierarchy
+* ```find_or_create_by_path``` for [building out heterogeneous hierarchies quickly and conveniently](#find_or_create_by_path)
+* Support for [deterministic ordering](#deterministic-ordering)
 * Support for [preordered](http://en.wikipedia.org/wiki/Tree_traversal#Pre-order) traversal of descendants
 * Support for rendering trees in [DOT format](http://en.wikipedia.org/wiki/DOT_(graph_description_language)), using [Graphviz](http://www.graphviz.org/)
-* Excellent [test coverage](#testing) in a variety of environments
+* Excellent [test coverage](#testing) in a comprehensive variety of environments
 
 See [Bill Karwin](http://karwin.blogspot.com/)'s excellent
 [Models for hierarchical data presentation](http://www.slideshare.net/billkarwin/models-for-hierarchical-data)
@@ -153,30 +153,40 @@ child1.ancestry_path
 
 ### find_or_create_by_path
 
-We can do all the node creation and add_child calls with one method call:
+You can ```find``` as well as ```find_or_create``` by "ancestry paths".
+
+If you provide an array of strings to these methods, they reference the `name` column in your 
+model, which can be overridden with the `:name_column` option provided to ```acts_as_tree```.
 
 ```ruby
 child = Tag.find_or_create_by_path(["grandparent", "parent", "child"])
 ```
 
-You can ```find``` as well as ```find_or_create``` by "ancestry paths".
-Ancestry paths may be built using any column in your model. The default
-column is ```name```, which can be changed with the :name_column option
-provided to ```acts_as_tree```.
+As of v5.0.0, `find_or_create_by_path` can also take an array of attribute hashes:
 
-Note that any other AR fields can be set with the second, optional ```attributes``` argument,
-and as of version 4.2.0, these attributes are added to the where clause as selection criteria.
+```ruby
+child = Tag.find_or_create_by_path([
+  {name: "Grandparent", title: "Sr."},
+  {name: "Parent", title: "Mrs."},
+  {name: "Child", title: "Jr."}
+])
+```
+
+If you're using STI, The attribute hashes can contain the `sti_name` and things work as expected:
 
 ```ruby
-child = Tag.find_or_create_by_path(%w{home chuck Photos"}, {:tag_type => "File"})
+child = Label.find_or_create_by_path([
+  {type: 'DateLabel', name: '2014'},
+  {type: 'DateLabel', name: 'August'},
+  {type: 'DateLabel', name: '5'},
+  {type: 'EventLabel', name: 'Visit the Getty Center'}
+])
 ```
-This will pass the attribute hash of ```{:name => "home", :tag_type => "File"}``` to
-```Tag.find_or_create_by_name``` if the root directory doesn't exist (and
-```{:name => "chuck", :tag_type => "File"}``` if the second-level tag doesn't exist, and so on).
 
 ### Moving nodes around the tree
 
-Nodes can be moved around to other parents, and closure_tree moves the node's descendancy to the new parent for you:
+Nodes can be moved around to other parents, and closure_tree moves the node's descendancy to the
+new parent for you:
 
 ```ruby
 d = Tag.find_or_create_by_path %w(a b c d)
@@ -252,6 +262,7 @@ When you include ```acts_as_tree``` in your model, you can provide a hash to ove
     * ```:destroy``` will destroy all descendant nodes (which runs the destroy hooks on each child node)
 * ```:name_column``` used by #```find_or_create_by_path```, #```find_by_path```, and ```ancestry_path``` instance methods. This is primarily useful if the model only has one required field (like a "tag").
 * ```:order``` used to set up [deterministic ordering](#deterministic-ordering)
+* ```:touch``` delegates to the `belongs_to` annotation for the parent, so `touch`ing cascades to all children (the performance of this for deep trees isn't currently optimal).
 
 ## Accessing Data
 
@@ -529,7 +540,6 @@ end
 
 ```
 
-
 ## Testing
 
 Closure tree is [tested under every valid combination](http://travis-ci.org/#!/mceachen/closure_tree) of

closure_tree.gemspec

@@ -8,27 +8,25 @@ Gem::Specification.new do |gem|
   gem.email       = ['matthew-github@mceachen.org']
   gem.homepage    = 'http://mceachen.github.io/closure_tree/'
 
-  gem.summary = %q(Easily and efficiently make your ActiveRecord model support hierarchies)
+  gem.summary     = %q(Easily and efficiently make your ActiveRecord model support hierarchies)
   gem.description = gem.summary
-  gem.license = 'MIT'
+  gem.license     = 'MIT'
 
-  gem.files         = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
-  gem.test_files    = gem.files.grep(%r{^spec/})
-  gem.required_ruby_version     = '>= 1.9.3'
+  gem.files       = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
+  gem.test_files  = gem.files.grep(%r{^spec/})
+  gem.required_ruby_version = '>= 1.9.3'
 
   gem.add_runtime_dependency 'activerecord', '>= 3.2.0'
-  gem.add_runtime_dependency 'with_advisory_lock', '>= 0.0.9' # <- to prevent duplicate roots
+  gem.add_runtime_dependency 'with_advisory_lock', '>= 3.0.0'
 
-  gem.add_development_dependency 'rake'
   gem.add_development_dependency 'yard'
   gem.add_development_dependency 'rspec', '>= 3.0'
   gem.add_development_dependency 'rspec-instafail'
+  # TODO: delete rspec-rails.
   gem.add_development_dependency 'rspec-rails' # FIXME: for rspec-rails and rspec fixture support
   gem.add_development_dependency 'uuidtools'
   gem.add_development_dependency 'database_cleaner'
   gem.add_development_dependency 'appraisal'
   gem.add_development_dependency 'timecop'
-
   # gem.add_development_dependency 'ruby-prof' # <- don't need this normally.
-
 end

lib/closure_tree/finders.rb

@@ -2,34 +2,34 @@ module ClosureTree
   module Finders
     extend ActiveSupport::Concern
 
-    # Find a child node whose +ancestry_path+ minus self.ancestry_path is +path+.
+    # Find a descendant node whose +ancestry_path+ will be ```self.ancestry_path + path```
     def find_by_path(path, attributes = {})
       return self if path.empty?
       self.class.find_by_path(path, attributes, id)
     end
 
-    # Find a child node whose +ancestry_path+ minus self.ancestry_path is +path+
-    def find_or_create_by_path(path, attributes = {}, find_before_lock = true)
-      attributes[:type] ||= self.type if _ct.subclass? && _ct.has_type?
-      # only bother trying to find_by_path on the first call:
-      (find_before_lock && find_by_path(path, attributes)) || begin
-        subpath = path.is_a?(Enumerable) ? path.dup : [path]
-        return self if subpath.empty?
-        child_name = subpath.shift
-        attrs = attributes.merge(_ct.name_sym => child_name)
-        _ct.with_advisory_lock do
-          # shenanigans because children.create is bound to the superclass
-          # (in the case of polymorphism):
-          child = self.children.where(attrs).first || begin
-            self.class.new(attrs).tap do |ea|
-              # We know that there isn't a cycle, because we just created it, and
-              # cycle detection is expensive when the node is deep.
-              ea._ct_skip_cycle_detection!
-              self.children << ea
-            end
+    # Find or create a descendant node whose +ancestry_path+ will be ```self.ancestry_path + path```
+    def find_or_create_by_path(path, attributes = {})
+      subpath = _ct.build_ancestry_attr_path(path, attributes)
+      return self if subpath.empty?
+
+      found = find_by_path(subpath, attributes)
+      return found if found
+
+      attrs = subpath.shift
+      _ct.with_advisory_lock do
+        # shenanigans because children.create is bound to the superclass
+        # (in the case of polymorphism):
+        child = self.children.where(attrs).first || begin
+          # Support STI creation by using base_class:
+          _ct.create(self.class, attrs).tap do |ea|
+            # We know that there isn't a cycle, because we just created it, and
+            # cycle detection is expensive when the node is deep.
+            ea._ct_skip_cycle_detection!
+            self.children << ea
           end
-          child.find_or_create_by_path(subpath, attributes, false)
         end
+        child.find_or_create_by_path(subpath, attributes)
       end
     end
 
@@ -54,7 +54,7 @@ module ClassMethods
 
       # Fix deprecation warning:
       def _ct_all
-        (ActiveRecord::VERSION::MAJOR >= 4 ) ? all : scoped
+        (ActiveRecord::VERSION::MAJOR >= 4) ? all : scoped
       end
 
       def without(instance)
@@ -115,49 +115,37 @@ def find_all_by_generation(generation_level)
         _ct.scope_with_order(s)
       end
 
-      def ct_scoped_attributes(scope, attributes, target_table = table_name)
-        attributes.inject(scope) do |scope, pair|
-          scope.where("#{target_table}.#{pair.first}" => pair.last)
-        end
-      end
-
       # Find the node whose +ancestry_path+ is +path+
       def find_by_path(path, attributes = {}, parent_id = nil)
-        path = path.is_a?(Enumerable) ? path.dup : [path]
-        scope = where(_ct.name_sym => path.pop).readonly(false)
-        scope = ct_scoped_attributes(scope, attributes)
+        path = _ct.build_ancestry_attr_path(path, attributes)
+        if path.size > _ct.max_join_tables
+          return _ct.find_by_large_path(path, attributes, parent_id)
+        end
+        scope = where(path.pop)
         last_joined_table = _ct.table_name
-        # MySQL doesn't support more than 61 joined tables (!!):
-        path.first(50).reverse.each_with_index do |ea, idx|
+        path.reverse.each_with_index do |ea, idx|
           next_joined_table = "p#{idx}"
           scope = scope.joins(<<-SQL.strip_heredoc)
             INNER JOIN #{_ct.quoted_table_name} AS #{next_joined_table}
               ON #{next_joined_table}.#{_ct.quoted_id_column_name} =
-                #{connection.quote_table_name(last_joined_table)}.#{_ct.quoted_parent_column_name}
+ #{connection.quote_table_name(last_joined_table)}.#{_ct.quoted_parent_column_name}
           SQL
-          scope = scope.where("#{next_joined_table}.#{_ct.name_column}" => ea)
-          scope = ct_scoped_attributes(scope, attributes, next_joined_table)
+          scope = _ct.scoped_attributes(scope, ea, next_joined_table)
           last_joined_table = next_joined_table
         end
-        result = scope.where("#{last_joined_table}.#{_ct.parent_column_name}" => parent_id).first
-        if path.size > 50 && result
-          find_by_path(path[50..-1], attributes, result.primary_key)
-        else
-          result
-        end
+        scope.where("#{last_joined_table}.#{_ct.parent_column_name}" => parent_id).readonly(false).first
       end
 
       # Find or create nodes such that the +ancestry_path+ is +path+
       def find_or_create_by_path(path, attributes = {})
-        find_by_path(path, attributes) || begin
-          subpath = path.dup
-          root_name = subpath.shift
+        attr_path = _ct.build_ancestry_attr_path(path, attributes)
+        find_by_path(attr_path) || begin
+          root_attrs = attr_path.shift
           _ct.with_advisory_lock do
             # shenanigans because find_or_create can't infer that we want the same class as this:
             # Note that roots will already be constrained to this subclass (in the case of polymorphism):
-            attrs = attributes.merge(_ct.name_sym => root_name)
-            root = roots.where(attrs).first || roots.create!(attrs)
-            root.find_or_create_by_path(subpath, attributes)
+            root = roots.where(root_attrs).first || _ct.create!(self, root_attrs)
+            root.find_or_create_by_path(attr_path)
           end
         end
       end